Skip to content

[AArch64] Clarify that Anyext is OK for MOPS instructions. NFC #70298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

davemgreen
Copy link
Collaborator

The instruction should only read the bottom 8 bits of the register, so an anyext is OK here. Update the comment from zext->anyext to clarify.

Closes #70270

The instruction should only read the bottom 8 bits of the register, so an
anyext is OK here. Update the comment from zext->anyext to clarify.

Closes llvm#70270
@llvmbot
Copy link
Member

llvmbot commented Oct 26, 2023

@llvm/pr-subscribers-backend-aarch64

Author: David Green (davemgreen)

Changes

The instruction should only read the bottom 8 bits of the register, so an anyext is OK here. Update the comment from zext->anyext to clarify.

Closes #70270


Full diff: https://github.com/llvm/llvm-project/pull/70298.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp (+4-2)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index 9fadc008de20628..6c1d25f62c21e76 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1253,7 +1253,8 @@ bool AArch64LegalizerInfo::legalizeIntrinsic(LegalizerHelper &Helper,
   }
   case Intrinsic::aarch64_mops_memset_tag: {
     assert(MI.getOpcode() == TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS);
-    // Zext the value to 64 bit
+    // Anyext the value being set to 64 bit (only the bottom 8 bits are read by
+    // the instruction).
     MachineIRBuilder MIB(MI);
     auto &Value = MI.getOperand(3);
     Register ZExtValueReg = MIB.buildAnyExt(LLT::scalar(64), Value).getReg(0);
@@ -1776,7 +1777,8 @@ bool AArch64LegalizerInfo::legalizeMemOps(MachineInstr &MI,
 
   // Tagged version MOPSMemorySetTagged is legalised in legalizeIntrinsic
   if (MI.getOpcode() == TargetOpcode::G_MEMSET) {
-    // Zext the value operand to 64 bit
+    // Anyext the value being set to 64 bit (only the bottom 8 bits are read by
+    // the instruction).
     auto &Value = MI.getOperand(1);
     Register ZExtValueReg =
         MIRBuilder.buildAnyExt(LLT::scalar(64), Value).getReg(0);

@tschuett
Copy link

The variable names still contain Zext.

davemgreen added a commit that referenced this pull request Oct 27, 2023
The instruction should only read the bottom 8 bits of the register, so an
anyext is OK here. Update the comment from zext->anyext to clarify.

Closes #70270 and #70298
@davemgreen davemgreen closed this Oct 28, 2023
@davemgreen davemgreen deleted the gh-gi-mopsanyext branch October 28, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suspicious anyextend in MOPS GlobalISel codegen
4 participants